Aggregate javadoc using build-logic #4457

runningcode · 2025-05-27T12:21:00Z

📜 Description

This PR has a few parts to it:

Adds a new build-logic module to organize our build logic.
Adds two convention plugins.
- sentry.javadoc publishes the javadoc
  to a consumable configuration.
- sentry.javadoc.aggregate consumes the published javadoc and
  aggregates them in the root build folder.
Deletes the stylesheet.css. This was causing the javadoc to look
unusable. Deleting it fixes this.
Each subproject publishes the javadoc to its own subfolder. The
previous behavior would overwrite the javadoc with each new project
in the same directory which meant some parts of the javadoc were
unreachable.

The producer/consumer configuration is based on this blog post: https://www.liutikas.net/2024/12/11/Together-In-Isolation.html
It should be project isolation compatible.

Here is what the javadoc looks like right now: https://getsentry.github.io/sentry-java/
This is what the javadoc will look like without the stylesheet:

💡 Motivation and Context

The previous mechanism for publishing javadoc was broken because it combines the classpath of all subproject dependencies leading to some version conflicts. This is why the publication was disabled here: #4445

💚 How did you test it?

I checked the build folder that the correct javadoc was published but wasn't able to test the github action.

📝 Checklist

I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
I updated the wizard if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

runningcode · 2025-05-27T12:22:21Z

build.gradle.kts

@@ -246,31 +247,6 @@ spotless {
    }
 }

-tasks.register("aggregateJavadocs", Javadoc::class.java) {
-    setDestinationDir(project.layout.buildDirectory.file("docs/javadoc").get().asFile)


This would previously put all the javadocs from all subprojects in the root which meant a lot of the files were overwritten like index.html.

github-actions · 2025-05-27T13:26:13Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	427.88 ms	480.38 ms	52.50 ms
Size	1.58 MiB	2.08 MiB	510.91 KiB

Previous results on branch: no/aggregate-javadoc

Startup times

Revision	Plain	With Sentry	Diff
`a151608`	459.64 ms	525.40 ms	65.76 ms
`b51509b`	415.16 ms	447.29 ms	32.13 ms
`283a4b4`	406.41 ms	475.78 ms	69.37 ms
`ba39ab5`	463.60 ms	488.32 ms	24.72 ms
`79d6db9`	389.84 ms	421.86 ms	32.02 ms

App size

Revision	Plain	With Sentry	Diff
`a151608`	1.58 MiB	2.08 MiB	510.84 KiB
`b51509b`	1.58 MiB	2.08 MiB	510.85 KiB
`283a4b4`	1.58 MiB	2.08 MiB	510.85 KiB
`ba39ab5`	1.58 MiB	2.08 MiB	510.84 KiB
`79d6db9`	1.58 MiB	2.08 MiB	510.84 KiB

runningcode · 2025-05-27T14:24:58Z

build.gradle.kts

@@ -237,7 +238,7 @@ spotless {
    kotlin {
        target("**/*.kt")
        ktlint()
-        targetExclude("**/sentry-native/**")
+        targetExclude("**/sentry-native/**", "**/build/**")


Had to add this exclusion otherwise spotless tries to format the generated code in the build folder. This was causing an issue since the generated accessors have an underscore (_) in the path name and this is was failing the check.

rbro112 · 2025-05-27T17:55:48Z

build-logic/src/main/kotlin/io/sentry/gradle/AggregateJavadoc.kt

+            fs.copy {
+                // Get the third to last part (project name) to use as the directory name for the output
+                val parts = file.path.split('/')
+                val projectName = parts[parts.size - 4]


Should there be some safety around this in case the parts.size is < 4?

This is a good point. To be honest, this logic is a bit hacky. The ArtifactView has this information as the component idenitifer, but I can’t have the ArtifactView as an input to the task since it isn’t serializeable. This was my hack around this.

Since these are fully qualified paths it is nearly impossible for them to be less than 4. If they are then we should fail and investigate. The bigger issue is if these are not the project names but I’m not sure how to check for that.

How about getting the root project folder (<path>/sentry-java) and treating that as a prefix which can be removed from file.path (<path>/sentry-java/module/build/docs/javadoc)?
Something along those lines:

file.path.replace(rootPath, "").split('/')[0]

I think there's also relativize or something in Kotlin which will return the path relative to root in this case

This is a good point. I can add the root directory to the task in order to compute the relative path.
Then we also need to remove the build/doc/javadoc from the end of the path. I will do this.

markushi

Looks great, thank you 🙏

markushi · 2025-06-02T05:53:41Z

build-logic/src/main/kotlin/io/sentry/gradle/AggregateJavadoc.kt

+            fs.copy {
+                // Get the third to last part (project name) to use as the directory name for the output
+                val parts = file.path.split('/')
+                val projectName = parts[parts.size - 4]


How about getting the root project folder (<path>/sentry-java) and treating that as a prefix which can be removed from file.path (<path>/sentry-java/module/build/docs/javadoc)?
Something along those lines:

file.path.replace(rootPath, "").split('/')[0]

markushi · 2025-06-02T06:00:37Z

build.gradle.kts

@@ -27,6 +27,7 @@ plugins {
    alias(libs.plugins.errorprone) apply false
    alias(libs.plugins.gradle.versions) apply false
    alias(libs.plugins.spring.dependency.management) apply false
+    id("sentry.javadoc.aggregate")


It's probably best to use the full io.sentry.** namespace instead of sentry.** for the plugin ids.

It’s a good point, i wanted to distinguish this internal plugin from externally published plugins. I’ll do the switch but maybe its worth discussing how to distinguish internal vs external plugins.

markushi · 2025-06-02T06:02:21Z

settings.gradle.kts

@@ -17,7 +17,7 @@ dependencyResolutionManagement {

 rootProject.name = "sentry-root"
 rootProject.buildFileName = "build.gradle.kts"
-
+includeBuild("build-logic")


Is it common convention to use dashes here? I know we have buildSrc(camel case) and I'm wondering if we should align the naming here.

Good point. I think the general convention in the gradle community is dashes. All our other projects in this sentry-java are also dashes.
Gradle’s sharing build logic sample also uses kebab case (aka dashes).

In addition name abbreviation works for both camel case and kebab case names.

CHANGELOG.md

romtsn · 2025-06-02T07:58:47Z

build-logic/src/main/kotlin/sentry.javadoc.gradle.kts

@@ -0,0 +1,27 @@
+val javadocConfig: Configuration by configurations.creating {


Suggested change

val javadocConfig: Configuration by configurations.creating {

val javadocPublisher: Configuration by configurations.creating {

maybe call it a publisher so it better aligns with consumer?

romtsn

LGTM, great findings and fixes 👍

This PR has a few parts to it: 1. Adds a new `build-logic` module to organize our build logic. 2. Adds two convention plugins. - `sentry.javadoc` publishes the javadoc to a consumable configuration. - `sentry.javadoc.aggregate` consumes the published javadoc and aggregates them in the root build folder. 3. Deletes the `stylesheet.css`. This was causing the javadoc to look unusable. Deleting it fixes this. 4. Each subproject publishes the javadoc to its own subfolder. The previous behavior would overwrite the javadoc with each new project in the same directory which meant some parts of the javadoc were unreachable. The producer/consumer configuration is based on this blog post: https://www.liutikas.net/2024/12/11/Together-In-Isolation.html It should be project isolation compatible.

runningcode requested review from adinauer, romtsn, stefanosiano, markushi and lcian as code owners May 27, 2025 12:21

runningcode commented May 27, 2025

View reviewed changes

runningcode force-pushed the no/aggregate-javadoc branch 3 times, most recently from f4724d7 to d4be83a Compare May 27, 2025 13:16

runningcode force-pushed the no/aggregate-javadoc branch 2 times, most recently from c2dbb1e to 2651164 Compare May 27, 2025 13:38

runningcode commented May 27, 2025

View reviewed changes

runningcode force-pushed the no/aggregate-javadoc branch from 2651164 to 3414ddc Compare May 27, 2025 14:26

rbro112 reviewed May 27, 2025

View reviewed changes

rbro112 approved these changes May 27, 2025

View reviewed changes

runningcode force-pushed the no/aggregate-javadoc branch 2 times, most recently from 1e77db3 to 9aecae9 Compare May 28, 2025 07:25

markushi approved these changes Jun 2, 2025

View reviewed changes

romtsn reviewed Jun 2, 2025

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

romtsn reviewed Jun 2, 2025

View reviewed changes

romtsn approved these changes Jun 2, 2025

View reviewed changes

runningcode added 3 commits June 2, 2025 06:54

Ignore .kotlin in all directories

e9209f6

Add back makefile and github workflow

8e54d1a

runningcode force-pushed the no/aggregate-javadoc branch from 9aecae9 to 4542e09 Compare June 2, 2025 14:11

Address review comments

9c33071

runningcode force-pushed the no/aggregate-javadoc branch from 4542e09 to 9c33071 Compare June 2, 2025 14:12

runningcode merged commit 6df3e18 into main Jun 2, 2025
33 checks passed

runningcode deleted the no/aggregate-javadoc branch June 2, 2025 16:59

		@@ -0,0 +1,27 @@
		val javadocConfig: Configuration by configurations.creating {

	val javadocConfig: Configuration by configurations.creating {
	val javadocPublisher: Configuration by configurations.creating {

Uh oh!

Aggregate javadoc using build-logic #4457

Aggregate javadoc using build-logic #4457

Uh oh!

Conversation

runningcode commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Previous results on branch: no/aggregate-javadoc

Startup times

App size

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markushi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

runningcode Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romtsn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

runningcode commented May 27, 2025 •

edited

Loading

github-actions bot commented May 27, 2025 •

edited

Loading

runningcode Jun 2, 2025 •

edited

Loading